[core] Generalize TROOT::GetIncludeDir() using build tree marker #20828
+52
−30
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guitargeek
added
in:Build System
in:Core Libraries
clean build
guitargeek
changed the title
FoundationUtils::GetIncludeDir() from RootSysTROOT::GetIncludeDir()
guitargeek
force-pushed
the
incpath_followup
branch
from
00c457f to
c0e43a8
Compare
guitargeek
changed the title
TROOT::GetIncludeDir()
Contributor
Author
|
Hi @ellert, I can't think of a better solution. What do you think? |
guitargeek
force-pushed
the
incpath_followup
branch
2 times, most recently
from
3801c9a to
a8725ae
Compare
pcanal
reviewed
Jan 9, 2026
guitargeek
force-pushed
the
incpath_followup
branch
from
a8725ae to
909168a
Compare
pcanal
reviewed
Jan 9, 2026
pcanal
reviewed
Jan 9, 2026
guitargeek
force-pushed
the
incpath_followup
branch
from
909168a to
5babc8c
Compare
Introducing a new marker file generalizes `TROOT::GetIncludeDir()` to work in all cases for both relocated install and build trees, even if the `CMAKE_INSTALL_INCLUDEDIR` is different from `include`. This follows up on 43ec084, which was incomplete and left ROOT configuration in a broken state, because `ROOTINCDIR` was not defined anymore but still used. The `RConfigure.h` header still defines a few other internal macros with hardcoded paths, and if the suggested mechanism proves to be stable for the include directory, we can also migrate the other macros. They are fragile anyway. For example, they contain relative paths for `gnuinstall=OFF` and absolute paths for `gnuinstall=ON`, which is nowhere documented and breaks relocatable builds. Also, they use `ROOTSYS` in the case of `gnuinstall=OFF`, which we want to migrate away from. The `ROOT::FoundationUtils::GetIncludeDir()` function was also removed and completely absorbed into `TROOT::GetIncludeDir()`, as it was not used in any other place. Also, allow absolute `CMAKE_INSTALL_INCLUDEDIR` paths.
guitargeek
force-pushed
the
incpath_followup
branch
from
5babc8c to
78364a5
Compare
Test Results 22 files 22 suites 3d 20h 9m 22s ⏱️ Results for commit 78364a5. |
1 task
Contributor
Author
|
@pcanal, thanks a lot for your review and also suggestions for future PRs! Is this PR as it is okay for you? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing a new marker file generalizes
TROOT::GetIncludeDir()towork in all cases for both relocated install and build trees, even if
the
CMAKE_INSTALL_INCLUDEDIRis different frominclude.This follows up on guitargeek@43ec084, which was incomplete and left ROOT
configuration in a broken state, because
ROOTINCDIRwas not definedanymore but still used.
The
RConfigure.hheader still defines a few other internal macros withhardcoded paths, and if the suggested mechanism proves to be stable for
the include directory, we can also migrate the other macros. They are
fragile anyway. For example, they contain relative paths for
gnuinstall=OFFand absolute paths forgnuinstall=ON, which isnowhere documented and breaks relocatable builds. Also, they use
ROOTSYSin the case ofgnuinstall=OFF, which we want to migrate awayfrom.
The
ROOT::FoundationUtils::GetIncludeDir()function was also removedand completely absorbed into
TROOT::GetIncludeDir(), as it was notused in any other place.
Also, allow absolute
CMAKE_INSTALL_INCLUDEDIRpaths.Follows up on:
ROOTSYSin TSystem include path #20823 (comment)